Repository navigation
Add TF-PSA-Crypto pk and threading API - #49
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Absolute BUILD_DIR paths break test compilation and the sibling wolfSSL rebuild, while Ed448-only builds underestimate the private-key export buffer size, causing psa_export_key() to fail.
2 open issues
What changed in this PR
Adds TF-PSA-Crypto private-key and pthread compatibility APIs to wolfPSA. Building on #40, it also generates feature macros from the active wolfCrypt configuration for standalone and Zephyr builds.
Changes:
- Adds RSA/EC private-key parsing, capability checks, mutexes, and condition variables.
- Shares feature-configuration generation across builds and expands advertised algorithms and buffer sizes.
- Adds API and feature-surface tests, CI coverage, and build documentation.
| File | Description |
|---|---|
| zephyr/gen/gen-crypto-config.py | Removes the Zephyr-specific generator. |
| zephyr/CMakeLists.txt | Uses shared generation tools and updated probe flags. |
| wolfpsa/tf-psa-crypto/build_info.h | Exposes PK and pthread feature macros. |
| wolfpsa/psa/crypto.h | Includes TF-PSA build information. |
| wolfpsa/psa/crypto_sizes.h | Extends Edwards and post-quantum buffer sizing. |
| wolfpsa/psa/crypto_extra.h | Clarifies callback cleanup documentation. |
| wolfpsa/psa/crypto_config.h | Loads generated standalone configuration. |
| wolfpsa/mbedtls/threading.h | Declares mutex and condition-variable APIs. |
| wolfpsa/mbedtls/pk.h | Declares private-key compatibility APIs. |
| wolfpsa.map | Exports new compatibility symbols. |
| user_settings.h | Enables thread safety with MBEDTLS_THREADING_C. |
| tools/psa_want_probe.c | Expands feature detection and emitted options. |
| tools/gen-crypto-config.py | Adds a shared configuration-header generator. |
| test/wolfcrypt-benchmark/Makefile | Includes generated configuration headers. |
| test/psa_server/psa_want_surface_test.c | Checks advertised features and maximum buffer sizes. |
| test/psa_server/psa_tf_psa_threading_test.c | Tests threading APIs and concurrent key access. |
| test/psa_server/psa_tf_psa_pk_test.c | Tests key parsing and capability checks. |
| test/Makefile | Integrates new tests and threaded library variants. |
| test-psa-api/psa-crypto-tests.sh | Adds the generated-header include directory. |
| src/psa_tf_psa.c | Implements PK and pthread compatibility APIs. |
| src/psa_key_storage.c | Guards an RSA-only error helper. |
| README.md | Documents PK support, threading, and generated headers. |
| Makefile | Adds configuration generation and threading build support. |
| .gitignore | Ignores new test and tooling artifacts. |
| .github/workflows/test-psa-api.yml | Runs feature-surface checks for ECC variants. |
| .github/workflows/build-config-matrix.yml | Expands configuration variants and surface checks. |
Review effort: Balanced
350ef4c to
f870e68
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The private-key parsing and threading compatibility layer relies on external TF-PSA-Crypto and wolfSSL contracts unavailable in this snapshot, leaving supported-version and backend integration validation outstanding.
No open issues 🎉
2 resolved since last review
Review effort: Balanced
Frauschi
left a comment
There was a problem hiding this comment.
Mostly places where the pk shim answers differently from TF-PSA-Crypto 1.x, plus two threading build gaps.
| PkCheckKey(pk->der, (word32)pk->der_len, &type) != 0) { | ||
| return 0; | ||
| } | ||
| if (type == PK_TYPE_RSA) { |
There was a problem hiding this comment.
TF-PSA imports a parsed RSA key with SIGN/VERIFY usage only and an EC key with SIGN only, so its mbedtls_pk_can_do_psa returns 0 for RSA ENCRYPT/DECRYPT and EC VERIFY_HASH. Please match that and update canDoCases 2, 3 and 7, or document the difference in pk.h.
There was a problem hiding this comment.
Done, looping over a table of valid options in mbedtls_pk_can_do_psa
| return PSA_ERROR_INSUFFICIENT_MEMORY; | ||
| } | ||
| idx = 0; | ||
| ret = PkRet(wc_RsaPrivateKeyDecode(der, &idx, rsa, len), |
There was a problem hiding this comment.
PkCheckKey never checks idx == len, so a DER key with trailing bytes parses as 0 and the junk stays in ctx->der. TF-PSA rejects trailing material with MBEDTLS_ERR_PK_KEY_INVALID_FORMAT; please do the same here.
There was a problem hiding this comment.
Added check in PkDecodeRet
| int type = PK_TYPE_NONE; | ||
|
|
||
| if (pk == NULL || pk->der == NULL || | ||
| PkCheckKey(pk->der, (word32)pk->der_len, &type) != 0) { |
There was a problem hiding this comment.
This decodes the whole private key on every can_do_psa call, and an EC key pays for a failed RSA decode first. Storing the PK_TYPE_* from mbedtls_pk_parse_key in mbedtls_pk_context would make this a lookup.
There was a problem hiding this comment.
Added type field as suggested
| { | ||
| int ret; | ||
|
|
||
| (void)pwd; |
There was a problem hiding this comment.
An encrypted key returns MBEDTLS_ERR_PK_KEY_INVALID_FORMAT and a supplied password is silently ignored, where TF-PSA returns MBEDTLS_ERR_PK_PASSWORD_REQUIRED. Please return PASSWORD_REQUIRED or FEATURE_UNAVAILABLE (both already in pk.h) so callers don't report a corrupt key.
There was a problem hiding this comment.
Updated to check for encrypted key and return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE
| /* MBEDTLS_ENTROPY_C and MBEDTLS_CTR_DRBG_C come from crypto_config.h */ | ||
| /* mbedtls/pk.h: private keys, PEM or DER */ | ||
| #define MBEDTLS_PK_C | ||
| #define MBEDTLS_PK_PARSE_C |
There was a problem hiding this comment.
MBEDTLS_PK_PARSE_C advertises the whole parse module, but only five pk functions exist, so code gated on it that calls mbedtls_pk_parse_public_key or mbedtls_pk_get_type fails to link. Please drop it unless a consumer needs it, and list the supported subset in pk.h.
There was a problem hiding this comment.
Updated comment, but this seems harmless while we only have 5 functions implemented. Lmk if you feel strongly
| mbedtls_pk_free; | ||
| mbedtls_pk_init; | ||
| mbedtls_pk_parse_key; | ||
| mbedtls_pk_parse_keyfile; |
There was a problem hiding this comment.
mbedtls_pk_parse_keyfile is only built with MBEDTLS_FS_IO, so in a NO_FILESYSTEM build this names an undefined symbol, which lld 16+ rejects by default. Please add a stub returning MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE, as psa_api_stub.c does for the PSA calls.
There was a problem hiding this comment.
Fixed using a wildcard in the map file rather than a stub
| } | ||
| #endif /* MBEDTLS_FS_IO */ | ||
|
|
||
| #if defined(MBEDTLS_THREADING_PTHREAD) |
There was a problem hiding this comment.
Only the root user_settings.h ties MBEDTLS_THREADING_C to WOLFPSA_THREAD_SAFE, so a custom USER_SETTINGS_PATH can export this API with an unlocked key store. Please add an #error for MBEDTLS_THREADING_C without WOLFPSA_THREAD_SAFE or with SINGLE_THREADED.
| $(VARIANT_INC) -I$(THREADS_BUILD_DIR)/include | ||
|
|
||
| $(THREADS_LIB): FORCE | ||
| $(call variant_lib,$(THREADS_BUILD_DIR),-DMBEDTLS_THREADING_C) |
There was a problem hiding this comment.
Setting -DMBEDTLS_THREADING_C through WOLFSSL_CPPFLAGS skips the top-level MBEDTLS_THREADING_C=1 branch (the stamp says options_nothreads), so CI never builds the documented path. Please pass MBEDTLS_THREADING_C=1 to the sub-make instead.
e94a2b8 to
8ff6c81
Compare

Merge #40 first.
Adds two parts of the TF-PSA api and header files.